feat(staging): render list bullets and title heading depth in markdown element_to_md() function#4384
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="unstructured/staging/base.py">
<violation number="1" location="unstructured/staging/base.py:283">
P1: The new Markdown heading logic is off-by-one because `metadata.category_depth` is zero-indexed in this codebase, but the code treats it as the literal Markdown heading level. For example, an h2 (category_depth=1) renders as `#` instead of `##`, and an h3 (category_depth=2) renders as `##` instead of `###`. This undermines the PR's goal of adding L2/L3 header support.
Also, the converted depth is not clamped to Markdown's valid 1-6 ATX heading range, so edge-case values like `0`, negatives, or values greater than 5 can produce invalid or oversized heading markers. Consider computing the Markdown depth as `max(1, min(category_depth + 1, 6))` and treating `None` as the top-level default (0).</violation>
<violation number="2" location="unstructured/staging/base.py:290">
P2: The new `ListItem` branch renders a Markdown list item by simply prepending `- ` to the text. If `ListItem.text` contains an embedded newline, the generated Markdown becomes `- first line\nsecond line`, where the continuation line starts at column 0 and is parsed as a separate paragraph rather than part of the list item. Since `ListItem` is a plain text element and the serializer does not normalize incoming text, a multiline value from a partitioner or deserialization will corrupt the Markdown output. Consider collapsing embedded newlines to spaces or indenting continuation lines so they stay inside the list item.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| case Title(text=text): | ||
| return f"# {text}" | ||
| case Title(text=text, metadata=metadata): | ||
| depth = metadata.category_depth if metadata.category_depth else 1 |
There was a problem hiding this comment.
P1: The new Markdown heading logic is off-by-one because metadata.category_depth is zero-indexed in this codebase, but the code treats it as the literal Markdown heading level. For example, an h2 (category_depth=1) renders as # instead of ##, and an h3 (category_depth=2) renders as ## instead of ###. This undermines the PR's goal of adding L2/L3 header support.
Also, the converted depth is not clamped to Markdown's valid 1-6 ATX heading range, so edge-case values like 0, negatives, or values greater than 5 can produce invalid or oversized heading markers. Consider computing the Markdown depth as max(1, min(category_depth + 1, 6)) and treating None as the top-level default (0).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At unstructured/staging/base.py, line 283:
<comment>The new Markdown heading logic is off-by-one because `metadata.category_depth` is zero-indexed in this codebase, but the code treats it as the literal Markdown heading level. For example, an h2 (category_depth=1) renders as `#` instead of `##`, and an h3 (category_depth=2) renders as `##` instead of `###`. This undermines the PR's goal of adding L2/L3 header support.
Also, the converted depth is not clamped to Markdown's valid 1-6 ATX heading range, so edge-case values like `0`, negatives, or values greater than 5 can produce invalid or oversized heading markers. Consider computing the Markdown depth as `max(1, min(category_depth + 1, 6))` and treating `None` as the top-level default (0).</comment>
<file context>
@@ -278,8 +279,16 @@ def element_to_md(
- case Title(text=text):
- return f"# {text}"
+ case Title(text=text, metadata=metadata):
+ depth = metadata.category_depth if metadata.category_depth else 1
+ if not isinstance(depth, int):
+ try:
</file context>
| except (TypeError, ValueError): | ||
| depth = 1 | ||
| return f"{'#' * depth} {text}" | ||
| case ListItem(text=text): |
There was a problem hiding this comment.
P2: The new ListItem branch renders a Markdown list item by simply prepending - to the text. If ListItem.text contains an embedded newline, the generated Markdown becomes - first line\nsecond line, where the continuation line starts at column 0 and is parsed as a separate paragraph rather than part of the list item. Since ListItem is a plain text element and the serializer does not normalize incoming text, a multiline value from a partitioner or deserialization will corrupt the Markdown output. Consider collapsing embedded newlines to spaces or indenting continuation lines so they stay inside the list item.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At unstructured/staging/base.py, line 290:
<comment>The new `ListItem` branch renders a Markdown list item by simply prepending `- ` to the text. If `ListItem.text` contains an embedded newline, the generated Markdown becomes `- first line\nsecond line`, where the continuation line starts at column 0 and is parsed as a separate paragraph rather than part of the list item. Since `ListItem` is a plain text element and the serializer does not normalize incoming text, a multiline value from a partitioner or deserialization will corrupt the Markdown output. Consider collapsing embedded newlines to spaces or indenting continuation lines so they stay inside the list item.</comment>
<file context>
@@ -278,8 +279,16 @@ def element_to_md(
+ except (TypeError, ValueError):
+ depth = 1
+ return f"{'#' * depth} {text}"
+ case ListItem(text=text):
+ return f"- {text}"
case Formula(text=text):
</file context>
aaronsteers
changed the title
element_to_md
aaronsteers
changed the title
element_to_mdelement_to_md() function
Hello! 👋
I wrote a custom json/dict-to-markdown mapper back in 2024 and found recently you have a built-in mapper now! This PR attempts to fill the parity/feature gap between your built-in native markdown mapper and my custom mapper, so that hopefully I can stop maintaining the custom one. 😀
Specifically, this adds:
# Headerinstead of## Headeror### Header)- item oneinstead ofitem one)